gh-144356: fix data race in setiter_len() under no-gil
#144357
+29
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
setiter_len() was reading so->used without atomic access while concurrent mutations update it atomically under Py_GIL_DISABLED. Use an atomic load for so->used to avoid a data race. This preserves the existing semantics of __length_hint__ while making the access thread-safe. Signed-off-by: Yongtao Huang <yongtaoh2022@gmail.com>
eendebakpt
reviewed
Jan 31, 2026
| for t in threads: | ||
| t.start() | ||
|
|
||
| stop.set() |
Contributor
There was a problem hiding this comment.
This means the threads will stop right after they have started. I would prefer the pattern that is used in some other tests in this file: set a constant NUM_LOOPS (determined so that the test < 0.1 seconds, but there still is a decent number of mutations)
| setiterobject *si = (setiterobject*)op; | ||
| Py_ssize_t len = 0; | ||
| if (si->si_set != NULL && si->si_used == si->si_set->used) | ||
| PySetObject *so = si->si_set; |
Contributor
There was a problem hiding this comment.
Here so is a borrowed reference to si->si_set. But si->si_set can be cleared in setiter_iternext (if the iterator is exhausted) outside the critical section.
This is a different mechanism than the corresponding issue, so maybe something to address in another PR. But solving both together is something to consider.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Long log:
setiter_len()was readingso->usedwithout atomic access while concurrentmutations update it atomically under Py_GIL_DISABLED.
Use an atomic load for
so->usedto avoid a data race. This preserves theexisting semantics of
__length_hint__while making the access thread-safe.Signed-off-by: Yongtao Huang yongtaoh2022@gmail.com